Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated hostname to hostIp and updated methods and configs accordingly. #290

Open
wants to merge 2 commits into
base: tombstone
Choose a base branch
from

Conversation

Jason011125
Copy link

#260
Improved hostname by changing it to hostIp, which represents its true usage.

@aspirer
Copy link
Collaborator

aspirer commented Aug 15, 2023

Hi @Jason011125
Your commit is good, but we also need to consider the issue of backward compatibility. The existing cluster yaml configuration file writes the hostname field. After upgrading to the curveadm version that includes your pr, an error will be reported when performing related operations. Therefore, we need to ensure the compatibility of the hostname configuration, and it is recommended to use the hostip field in the configuration file of the new version, and discard the hostname field after several versions.

Can you continue to improve this PR?

@Wine93
Copy link
Collaborator

Wine93 commented Aug 20, 2023

Hi @Jason011125 Your commit is good, but we also need to consider the issue of backward compatibility. The existing cluster yaml configuration file writes the hostname field. After upgrading to the curveadm version that includes your pr, an error will be reported when performing related operations. Therefore, we need to ensure the compatibility of the hostname configuration, and it is recommended to use the hostip field in the configuration file of the new version, and discard the hostname field after several versions.

Can you continue to improve this PR?

Agree! On the other hand, i think use name to replace host and reserve hostname is a good choise, e.g.

hosts:
  - name: server1
    hostname: 10.0.1.1

OR

hosts:
  - name: server1
    hostname: server1.163.org

@Jason011125
Copy link
Author

Hey guys, can we take another look at the PR as I amend the hostip part to allow upward compatability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants